Skip to content

SOLR-18211 Migrate jwt-auth module from bc-jose4j to nimbus-jose-jwt#4334

Draft
janhoy wants to merge 12 commits intoapache:mainfrom
janhoy:feature/migrate-jwt-nimbus
Draft

SOLR-18211 Migrate jwt-auth module from bc-jose4j to nimbus-jose-jwt#4334
janhoy wants to merge 12 commits intoapache:mainfrom
janhoy:feature/migrate-jwt-nimbus

Conversation

@janhoy
Copy link
Copy Markdown
Contributor

@janhoy janhoy commented Apr 25, 2026

https://issues.apache.org/jira/browse/SOLR-18211

Motivation

org.bitbucket.b_c:jose4j is unmaintained — its last release was March 2024 and it is a single-maintainer project. com.nimbusds:nimbus-jose-jwt is a well-maintained, widely-used JOSE/JWT library that was already present as a
test dependency in this module.

Changes

JWTIssuerConfig

  • Replace JsonWebKeySet / JsonWebKey with nimbus JWKSet / JWK
  • Replace HttpsJwks with a new inner class JwkSetFetcher wrapping nimbus's ResourceRetriever with synchronized caching and a configurable TTL
  • Custom ResourceRetriever implementation preserves existing behaviour: trusted SSL certificates and hostname verification bypass for localhost endpoints

IssuerAwareJWSKeySelector (previously JWTVerificationkeyResolver)

  • Renamed to reflect the nimbus interface it now implements (JWSKeySelector<SecurityContext>)
  • Added IssuerContext implements SecurityContext to thread the JWT's (unverified) issuer claim from the payload to the key selector, enabling per-issuer key lookup
  • Key selection uses JWKSelector + JWKMatcher.forJWSHeader() to match on kid and algorithm

JWTAuthPlugin

  • Replace JwtConsumer / JwtConsumerBuilder with DefaultJWTProcessor<SecurityContext> + DefaultJWTClaimsVerifier
  • JWT is pre-parsed (without signature verification) to extract the issuer and enforce the algorithm allowlist before full processing
  • Explicit issuer value validation: when at least one issuer is configured with an iss value, the token's issuer must match

Testing

All 65 existing tests pass unchanged (1 skipped). Passes forbiddenApis, ecjLint, spotlessCheck, and validateLogCalls.

To further lower the risk of this migration, 13 new unit tests are added across the three test classes:

  • IssuerAwareJWSKeySelectorTest — covers all previously untested branches of resolveIssuerConfig(): null issuer with single multiple issuers, unrecognised issuer with single/multiple issuers, and correct materialisation of EC keys as ECPublicKey
  • JWTAuthPluginTest — covers: requireIssuer=false does not bypass issuer value validation; requireIssuer=false with no issuer in token or config authenticates successfully; scope claim expressed as a JSON array; asConfig() JWK round-trip produces a functional plugin; clock-skew tolerance (25 s expired → authenticated; 35 s expired → JWT_EXPIRED)
  • JWTIssuerConfigTest — covers: parseJwkSet with a bare single-JWK map (no "keys" wrapper); JwkSetFetcher refresh reprieve suppressing a second call within the window; JwkSetFetcher cache expiry triggering a re-fetch

Docs

While reviewing the code for this migration, several bugs were found in jwt-authentication-plugin.adoc and fixed:

  • blockUnknown default is false, not true — the ref guide has documented the default as true since apache/lucene-solr#805 (SOLR-13649), a docs-only PR merged in 9.0 that appears to have introduced the wrong default. The code has always used getOrDefault(PARAM_BLOCK_UNKNOWN, false), meaning unauthenticated requests pass through unless blockUnknown is explicitly set to true. The intro paragraph ("The plugin will by default require a valid JWT for all traffic") has also been corrected to match.
  • aud default — documented as "Uses clientId if configured"; the code has no such fallback. If aud is not set, audience validation is skipped entirely.
  • adminUiScope fallback — documented as falling back to the first scope entry; the code has a second fallback to the hardcoded string "solr" that was not mentioned.
  • Clock skew tolerance — a 30-second exp tolerance is applied by the plugin but was not documented anywhere; added a note to the requireExp row.

Disclaimer: This migration is done with help of Claude Code.

janhoy added 3 commits April 25, 2026 18:28
The old name reflected the jose4j VerificationKeyResolver interface it
used to implement. The class now implements JWSKeySelector<SecurityContext>
from nimbus-jose-jwt, with per-issuer key lookup as its key feature.
Also add JIRA link to changelog entry.

This comment was marked as resolved.

janhoy added 4 commits April 25, 2026 19:47
The refreshReprieveThreshold was stored on HttpsJwksFactory but never
passed to JwkSetFetcher, making it dead code. Repeated unknown-kid
requests would trigger a remote refresh on every auth attempt.

Wire the threshold through to JwkSetFetcher and track lastRefreshTime:
forced refresh() calls within the reprieve window are now skipped.
…ormat

asConfig() was storing the JWK as a flat List<Map>, but setJsonWebKeySet(Object)
expects a Map (JWK Set with 'keys' field or single JWK map). A config edit
round-trip would cause a ClassCastException on reload.

Wrap the key list in the standard JWK Set format {"keys": [...]} so it
round-trips correctly through parseJwkSet().
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTIssuerConfig.java Outdated
janhoy added 5 commits April 25, 2026 21:05
requireIssuer controls whether the iss claim must be present in the token.
It should not control whether a mismatching iss value is silently accepted
when the claim is present. A token whose iss doesn't match any configured
issuer should always be rejected if issuers are explicitly configured.
…path

SolrException (unchecked) thrown inside selectJWSKeys() escapes the
Nimbus DefaultJWTProcessor without being caught, producing an unexpected
500 instead of a JWT validation failure. Use KeySourceException so it
propagates as a JOSEException and is handled cleanly.
13 new tests across IssuerAwareJWSKeySelectorTest, JWTAuthPluginTest,
and JWTIssuerConfigTest covering: all resolveIssuerConfig() branches,
EC key materialisation, requireIssuer=false semantics, scope-as-array,
asConfig() JWK round-trip, clock-skew tolerance, JwkSetFetcher cache
expiry, and refresh reprieve suppression.

Doc fixes in jwt-authentication-plugin.adoc:
- blockUnknown default corrected from true to false (code has always
  used getOrDefault(..., false); the wrong default was introduced by a
  docs-only change in apache/lucene-solr#805 / SOLR-13649)
- aud default corrected: no clientId fallback exists in the code;
  audience validation is simply skipped when aud is not configured
- adminUiScope: document the second fallback to hardcoded "solr"
- requireExp: document the 30-second clock-skew tolerance
The table entry now clearly states the default is false and explains
both use cases (enforcement vs. open configuration). The intro paragraph
is rewritten as a single sentence that leads with the default behaviour.
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Apr 25, 2026
@janhoy janhoy marked this pull request as draft April 26, 2026 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cat:security dependencies Dependency upgrades documentation Improvements or additions to documentation module:jwt-auth tests tool:build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants